Skip to content

Conversation

gemmaro
Copy link

@gemmaro gemmaro commented Jan 1, 2025

Using TypeProf as a library does not resolve Gemfile dependencies. In particular, TypeProf uses the start_code_units_column method introduced in Prism version 0.23.0, so it must be at least that version or later.

* Gemfile, typeprof.gemspec: Using TypeProf as a library does not resolve
Gemfile dependencies.  In particular, TypeProf uses the
start_code_units_column method introduced in Prism version 0.23.0, so it must
be at least that version or later.
@sinsoku
Copy link
Contributor

sinsoku commented Oct 6, 2025

@gemmaro
The test failed on my machine too.
Adding the dependency to typeprof.gemspec looks good, but I think we need to keep the dependency in the Gemfile.
This is because gemspec is commented out in the Gemfile, and the gemspec dependencies are not being loaded.
https://github.com/ruby/typeprof/blob/24cf28ed49a8e82d289e5638e092e10ba48007dd/Gemfile#L7C4-L7C11

My guess is that this is intentionally not being loaded to prevent the typeprof version from being included in Gemfile.lock.

@gemmaro
Copy link
Author

gemmaro commented Oct 6, 2025

@sinsoku I understand your point. Since TypeProf is a gem used as an application rather than a library, it makes sense to keep things as they are. I'll go ahead and close this then.

@gemmaro gemmaro closed this Oct 6, 2025
@gemmaro gemmaro deleted the prism branch October 6, 2025 23:52
@sinsoku
Copy link
Contributor

sinsoku commented Oct 9, 2025

@gemmaro
I'm sorry for the confusion. My previous comment wasn't clear enough.

I think adding prism to the gemspec is a good change. What I meant to suggest was:

  • Add spec.add_runtime_dependency "prism", ">= 1.4.0" to typeprof.gemspec
  • Keep gem "prism", ">= 1.4.0" in the Gemfile as is (no changes to Gemfile)

Having the dependency in both places ensures it works properly for both gem users and development environments.

Note: The prism version requirement was updated to ">= 1.4.0" in #312.

Would you like to reopen the PR with just the gemspec change?

@gemmaro
Copy link
Author

gemmaro commented Oct 10, 2025

Thank you for the clarification.
I’m happy to contribute any changes that would be beneficial for TypeProf.

Your suggested modification sounds good to me. However, before proceeding, I’d like to briefly ask the repository members about how they prefer to manage dependencies and respect their intention.

I’ll mention @mame to confirm -- sorry in advance for my noise -- whether the decision not to load the gemspec from the Gemfile was indeed as @sinsoku explained, and whether the proposed approach would be acceptable.

Also, thank you for sharing that the version requirement for Prism was updated. I appreciate it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants